[Badger] Refactor approvals to use badger batch updates #6466
Conversation
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #6466 +/- ##
==========================================
+ Coverage 41.44% 41.47% +0.03%
==========================================
Files 2026 2042 +16
Lines 144629 145107 +478
==========================================
+ Hits 59937 60190 +253
- Misses 78492 78699 +207
- Partials 6200 6218 +18
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
| @@ -201,7 +203,7 @@ func (v *VerificationNodeBuilder) LoadComponentsAndModules() { | |||
| vmCtx := fvm.NewContext(fvmOptions...) | |||
|
|
|||
| chunkVerifier := chunks.NewChunkVerifier(vm, vmCtx, node.Logger) | |||
| approvalStorage := badger.NewResultApprovals(node.Metrics.Cache, node.DB) | |||
| approvalStorage := store.NewResultApprovals(node.Metrics.Cache, badgerimpl.ToDB(node.DB)) | |||
There was a problem hiding this comment.
Switching from badger to pebble in the future would just be changing badgerimpl to pebbleimpl here.
| @@ -1,31 +0,0 @@ | |||
| package operation | |||
There was a problem hiding this comment.
This file is moved to storage/operation/approvals.go, however git is not able to recognize it as a git file rename, still consider it as a deletion here.
| @@ -1,35 +1,33 @@ | |||
| package badger | |||
| package store | |||
There was a problem hiding this comment.
GitHub correctly recognized the changes are actually on top of a file renaming, much easier to view the actual changes. And there is no pebble version to review, because this is a universal one.
zhangchiqing
force-pushed
the
leo/db-ops-approval
branch
from
938cf9a to
299f064
Compare
zhangchiqing
force-pushed
the
leo/db-ops-approval
branch
from
1a176ff to
ebb01d8
Compare
zhangchiqing
force-pushed
the
leo/db-ops
branch
from
87dc8e1 to
b82106a
Compare
zhangchiqing
force-pushed
the
leo/db-ops-approval
branch
from
ebb01d8 to
87b20dd
Compare
zhangchiqing
force-pushed
the
leo/db-ops
branch
7 times, most recently
from
77b44d1 to
17972e0
Compare
Please review #6465 first.
The first attempt of the refactor refactored the approvals from using badger transaction to badger batch updates, allowing us to transit to pebble based implementation easier. However, it would still require keeping two implementations of the operations functions (storage/badger/operation/approvals.go and storage/pebble/operation/approvals.go) and corresponding storage modules.
This PR builds on top of the universal database operations, and implements only one version of database operations and store module. Easier to maintain than having two versions. Also makes it easy to switch to pebble version (explained in review comments).